Add ratelimiting #345
Add ratelimiting #345
Conversation
paramsingh
force-pushed
the
add-ratelimiting
branch
2 times, most recently
from
fa00500
to
bd24f62
Compare
manage.py
Outdated
| @click.argument('per_ip') | ||
| @click.argument('per_token') | ||
| @click.argument('window_size') | ||
| def set_rate_limits(per_ip, per_token, window_size): |
There was a problem hiding this comment.
we don't have token auth in AB, so how about we remove this parameter (and optionally set it to the same as per_ip?)
Please add documentation and types to the arguments.
It'd make sense to also print what the parameters are set to when you run this function.
Do we want any sanity checking to make sure that these settings aren't set to anything really strict? e.g. a warning if the effective limit goes lower than 1 per second
|
|
||
| The AcousticBrainz API is rate limited via the use of rate limiting headers that | ||
| are sent as part of the HTTP response headers. Each call will include the | ||
| following headers: |
There was a problem hiding this comment.
What's the default rate limit if we don't make any changes? It'd be nice to say this here. "We typically set the limit to 10 queries every 10 seconds, but these values may change. Make sure you check the response headers if you want to know the specific values"
There was a problem hiding this comment.
looking at the BU code it seems like the default is 30 queries in 10 seconds. Is this actually useful for us - should we check the mean number of queries per IP address for AB to ensure that this actually results in a reduction of queries, or see if we need to reduce the default.
Our custom defaults should be in the config file instead of hard-coded.
|
|
||
| bp_core = Blueprint('api_v1_core', __name__) | ||
|
|
||
|
|
||
| @bp_core.route("/<uuid:mbid>/count", methods=["GET"]) | ||
| @crossdomain() | ||
| @ratelimit() |
There was a problem hiding this comment.
I'd like to see at least one test to see that we're returning ratelimiting headers on one of these methods
paramsingh
force-pushed
the
add-ratelimiting
branch
from
d4a24f2
to
deb8fbd
Compare
paramsingh
force-pushed
the
add-ratelimiting
branch
from
fc3bf98
to
88db8bc
Compare
paramsingh
force-pushed
the
add-ratelimiting
branch
from
88db8bc
to
e664617
Compare
Remove the per_token argument. Also, add documentation and better logging.
Add ratelimiting default examples to example config file
paramsingh
force-pushed
the
add-ratelimiting
branch
from
5a28e1a
to
4b89e6e
Compare
Deployment steps
ratelimit_per_ip(100) andratelimit_window(10) key toconfig.prod.jsonindocker-server-configs/consul/AB